Repository navigation
fix(cohorts): stop rescanning every team cohort on each cohort write - #95271
Conversation
|
😎 Merged successfully - details. |
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 2 should fix, 1 consider. Published 3 findings (view the review). |
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/cohorts/backend/models/dependencies.py |
92.5% | 301, 310–311, 333–334, 469, 716–717 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 33911573234 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
warehouse_sources_queue |
██████░░░░░░░░░░░░░░ 29.1% |
92 / 316 |
demo |
███████████░░░░░░░░░ 57.0% |
1,523 / 2,671 |
data_tools |
█████████████░░░░░░░ 66.2% |
90 / 136 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
batch_exports |
████████████████░░░░ 79.8% |
19,411 / 24,336 |
apm |
█████████████████░░░ 83.3% |
1,234 / 1,481 |
signals |
█████████████████░░░ 85.2% |
35,553 / 41,713 |
cdp |
█████████████████░░░ 85.3% |
4,332 / 5,077 |
wizard |
█████████████████░░░ 87.3% |
2,105 / 2,411 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
notebooks |
██████████████████░░ 88.4% |
11,801 / 13,346 |
data_warehouse |
██████████████████░░ 88.7% |
12,732 / 14,359 |
business_knowledge |
██████████████████░░ 89.1% |
4,440 / 4,984 |
cohorts |
██████████████████░░ 89.2% |
7,526 / 8,433 |
engineering_analytics |
██████████████████░░ 89.5% |
8,163 / 9,124 |
dashboards |
██████████████████░░ 89.6% |
6,818 / 7,606 |
tasks |
██████████████████░░ 90.1% |
64,986 / 72,090 |
streamlit_apps |
██████████████████░░ 90.2% |
2,500 / 2,773 |
data_modeling |
██████████████████░░ 90.4% |
9,349 / 10,346 |
links |
██████████████████░░ 90.6% |
183 / 202 |
managed_warehouse |
██████████████████░░ 90.7% |
9,342 / 10,299 |
exports |
██████████████████░░ 90.7% |
9,120 / 10,051 |
alerts |
██████████████████░░ 90.9% |
5,224 / 5,745 |
visual_review |
██████████████████░░ 91.0% |
6,896 / 7,577 |
canvas |
██████████████████░░ 91.1% |
4,934 / 5,414 |
conversations |
██████████████████░░ 91.2% |
21,436 / 23,513 |
mcp_analytics |
██████████████████░░ 91.2% |
4,401 / 4,827 |
error_tracking |
██████████████████░░ 91.8% |
13,335 / 14,534 |
notifications |
██████████████████░░ 92.4% |
1,110 / 1,201 |
early_access_features |
███████████████████░ 92.5% |
1,323 / 1,430 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
slack_app |
███████████████████░ 92.8% |
12,596 / 13,578 |
surveys |
███████████████████░ 93.1% |
6,170 / 6,630 |
mcp_registry |
███████████████████░ 93.2% |
1,463 / 1,570 |
web_analytics |
███████████████████░ 93.2% |
18,857 / 20,230 |
context_layer |
███████████████████░ 93.3% |
2,593 / 2,779 |
stamphog |
███████████████████░ 93.3% |
6,533 / 6,999 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
marketing_analytics |
███████████████████░ 93.5% |
17,337 / 18,541 |
posthog_ai |
███████████████████░ 93.5% |
1,385 / 1,481 |
mcp_store |
███████████████████░ 93.5% |
8,053 / 8,611 |
ai_observability |
███████████████████░ 93.8% |
18,195 / 19,403 |
approvals |
███████████████████░ 93.9% |
3,560 / 3,793 |
billing_alerts |
███████████████████░ 94.0% |
2,069 / 2,200 |
endpoints |
███████████████████░ 94.2% |
8,875 / 9,425 |
review_hog |
███████████████████░ 94.3% |
10,203 / 10,815 |
tracing |
███████████████████░ 94.5% |
2,709 / 2,868 |
legal_documents |
███████████████████░ 94.7% |
2,073 / 2,190 |
workflows |
███████████████████░ 94.7% |
11,779 / 12,433 |
access_control |
███████████████████░ 94.9% |
6,052 / 6,378 |
customer_analytics |
███████████████████░ 95.1% |
19,157 / 20,151 |
annotations |
███████████████████░ 95.2% |
795 / 835 |
autoresearch |
███████████████████░ 95.3% |
443 / 465 |
product_analytics |
███████████████████░ 95.3% |
19,348 / 20,304 |
actions |
███████████████████░ 95.4% |
740 / 776 |
replay_vision |
███████████████████░ 95.5% |
22,574 / 23,642 |
skills |
███████████████████░ 95.6% |
5,434 / 5,686 |
experiments |
███████████████████░ 95.6% |
30,050 / 31,434 |
messaging |
███████████████████░ 95.8% |
3,671 / 3,830 |
logs |
███████████████████░ 95.9% |
13,712 / 14,294 |
growth |
███████████████████░ 95.9% |
7,699 / 8,025 |
feature_flags |
███████████████████░ 96.3% |
20,303 / 21,089 |
revenue_analytics |
███████████████████░ 96.3% |
1,841 / 1,911 |
user_interviews |
███████████████████░ 96.5% |
2,639 / 2,735 |
warehouse_sources |
███████████████████░ 97.2% |
408,942 / 420,831 |
data_quality |
████████████████████ 97.5% |
4,217 / 4,323 |
analytics_platform |
████████████████████ 98.1% |
2,286 / 2,330 |
metrics |
████████████████████ 98.2% |
3,564 / 3,628 |
data_catalog |
████████████████████ 98.4% |
3,256 / 3,310 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
Problem
Creating, deleting, or editing one cohort costs time proportional to how many cohorts the project already holds, and that work runs inside the web request.
warm_team_cohort_dependency_cacheinline, throughtransaction.on_commitwith noATOMIC_REQUESTS.filtersand does three to four Redis round trips per cohort.Changes
_on_cohort_changedwrites only the changed cohort's own keys and deletes the reverse lists of the cohorts it started or stopped referencing. DEL is idempotent, so concurrent writers cannot lose an update, and the next reader rebuilds the list.dependentsstill warms the team, but the warm loads onlyfiltersandgroups, writes withset_many, and never runs from a write path.cohort_dependencies, mirroringorganization_access. The create path reads its own keys back in the same request, and a replica-lag miss would rescan the team.COHORT_DEPENDENCY_INCREMENTAL_MAINTENANCE(default on) falls back to the full warm. A wrong incremental update shows only as a shorter recalculation chain, which no dashboard surfaces, so a config flip has to beat a revert.get_cohort_dependentsno longer filters the team lookup ondeleted, so a soft-deleted cohort with live dependents stops caching an empty list for a week.posthog_cohort_dependency_maintenance_total{operation,path}and the histogramposthog_cohort_dependency_warm_cohorts_scanned.Note
Residual staleness: an update whose cached
dependencieskey expired leaves the cohort in a former dependency's reverse list until the next warm. Consumers filter extra ids out. Missing ids, the case consumers do not tolerate, are never produced by this path.How did you test this code?
test_dependencies.pycases pass with one helper reordered (the raw reverse-key assertion moves after the read that rebuilds it) and the TTL test rewritten forset_many.dependencieskey absent still invalidates; soft delete then restore re-adds the reverse edge; a save without definition fields skips maintenance; the warm uses batched writes only; the reverse map spans batches; the kill switch restores the full warm.test_cohort_update_recalculated_after_caching, the dependency-chain tests intest_calculate_cohort.py, the dependency tests intest_util.py,hogli lint:tach, repo-wide mypy, andhogli ci:preflight.test_cohort.pyandtest_calculate_cohort.pysuites.Docs update
No.